Should address issue: #1608

The issue in #1608 is actually occurring the boards changed in #1604 (@tlaurion is working on that). On Librem boards, cbmem does segfault, but it emits the complete and correct TPM log before faulting, so Heads functionality works fine. I had not noticed it was faulting until it was mentioned in #1604.

That said I'm happy to test this and see if it addresses the segfault (may be a bit before I can get to it).

I had no problem dumping TPM log in TCG format with cbmem on 4.21+ too, but segfault is something new to me.

cbmem -L will also fail to dup the log if the first event is not SpecID event for TCG format. Not sure about that, but there might be a case where the first entry in the log is not SpecID event, i.e. vboot. If vboot is used (as separate stage IIRC) it may happen that measurement of verstage is first, then CRTM initialization and SpecID event is created later... I would have to double check.

But other than that, the SpecID should be always first if vboot is not used (which should be true for all heads builds). Maybe if preram TCG LOG is full, then something is being overwritten?

Created a branch I've built on talos II trying to make CircleCI happy which is happening under https://app.circleci.com/pipelines/github/tlaurion/heads?branch=pre423-edp_fhd_in-tpm_tcg_event_log-CircleCi_coreboot_cache_fixed which builds https://github.com/tlaurion/heads/tree/pre423-edp_fhd_in-tpm_tcg_event_log-CircleCi_coreboot_cache_fixed
EDIT: builds successfully there. @miczyg1 please reuse for testing. Building util/cbmem from different coreboot releases on ubuntu shows same segfault as per Heads cbmem, showing its not linked to cbmem being built with musl-cross-make, while coreboot is built with coreboot buildstack which should be working. So not sure why your behavior would be any different unless your buildsystem is not the one of coreboot (which I think is the case).

This changes coreboot config files to reflect changes needed to pass to 4.23 have TPM TCG Event log format, which was used to produce logs on x230 from cbmem built from musl-cross-make. As said under #1604, the errors reported there were for coreboot custom Event Log format, which was default before and where cbmem doesn't seem to have evolved correctly to support changes that happened after TCG addition.

Here, we built 4.23+ from commit including edp/fhd patch yersterday to reflect master state. The PR of course includes transformed coreboot configurations. Notes that I deactivated microcode inclusion there simply because git forks seem to change where blobs are and for the exercice, they were not relevant and made the build fails because not found in expected paths.

@miczyg1 Here are the logs produced from Heads output of cbmem -L (cbmem built from util/cbmem in tree)
cbmem_L_segfault.log
dmesg_cbmem_L_segfault.log

As said under #1604, building cbmem from tarball archives (4.19,4.20,4.21,4.22) produces invalid results. Here, since we switched to TCG under coreboot, the results are different. Here is raw output from my ubuntu testing station:
tcg_cbmem-L_parsing_across_coreboot_versions_atop_ubuntu.log
Of course, new format not recognized under 4.19 nor 4.20.1. And segfaults there for TPM1. Unfortunately, I could not get qemu to output cbmem, will retry and update when successful.

Notes:

• to build 4.23+ I had to add -Wno-error=array-bounds as reported under 213b44b
• past analysis of cbmem changes at 4.22.01 fhd patch merged upstream (4.24) + bump all 4.19 boards to 4.22.01 #1604 (comment)
• cbmem behavior of 4.21+ with coreboot custom TCPA as previoulsy configured in all Heads boards (but Talos) 4.22.01 fhd patch merged upstream (4.24) + bump all 4.19 boards to 4.22.01 #1604 (comment)

So not sure why your behavior would be any different unless your buildsystem is not the one of coreboot (which I think is the case).

Always using the coreboot toolchain inside docker. In case of heads we also have docker to get all cross toolchains to be built properly, but that still uses heads-built musl-cross and coreboot toolchain still...

TCPA log entry 10:
	PCR: -967035966
	Event type: Unknown (0xace82028 >= 19)
	Digest: d97f7b94ead60f73575cdf71010ef9fd99ed6b28
	Event data: 

Looks like coreboot didn't terminate the event log... cbmem checks for a zero_block of one TCPA entry length, if it isn;t found, then it goes further... We could add more safety checks to cbmem to avoid that but also make sure coreboot properly terminates the eventlog (or at least clear the cbmem entry with the log while creating it...)

@SergiiDmytruk cc

EDIT:

coreboot is clearing the cbmem areas with TCG logs in src/security/tpm/tspi/log-tpm1.c and src/security/tpm/tspi/log-tpm2.c: memset(tclt, 0, tpm_log_len);, so a memory corruption?

Of course, new format not recognized under 4.19 nor 4.20.1. And segfaults there for TPM1.

Each log is passed in a separate CBMEM entry, old versions of cbmem should just not find the log and be done.

coreboot is clearing the cbmem areas with TCG logs in src/security/tpm/tspi/log-tpm1.c and src/security/tpm/tspi/log-tpm2.c: memset(tclt, 0, tpm_log_len);, so a memory corruption?

Memory issue or some other bug is always possible. Endianness should be fine (inverting bytes doesn't seem to result in anything sensible). But given that the entry follows Event data: CBFS: fallback/payload, maybe something other than coreboot added it?

If vboot is used (as separate stage IIRC) it may happen that measurement of verstage is first, then CRTM initialization and SpecID event is created later... I would have to double check.

It isn't appending of bytes after bytes, those entries are copied individually into the area allocated for entries:

void tpm1_log_copy_entries(const void *from, void *to)
{
	const struct tpm_1_log_table *from_log = from;
	struct tpm_1_log_table *to_log = to;
	int i;

	for (i = 0; i < le16toh(from_log->vendor.num_entries); i++) {
		if (le16toh(to_log->vendor.num_entries) >= le16toh(to_log->vendor.max_entries)) {
			printk(BIOS_WARNING, "TPM LOG: log table is full\n");
			return;
		}

		struct tpm_1_log_entry *tce =
			&to_log->entries[le16toh(to_log->vendor.num_entries)];
		memcpy(tce, &from_log->entries[i], sizeof(*tce));

		to_log->vendor.num_entries = htole16(le16toh(to_log->vendor.num_entries) + 1);
	}
}

That area is after the header which contains SpecID:

struct tpm_1_log_table {
	/* The first entry of the log is inlined and describes the log itself */
	uint32_t pcr;
	uint32_t event_type;
	uint8_t digest[TPM_1_LOG_DIGEST_MAX_LENGTH];
	uint32_t spec_id_size;
	struct spec_id_event_data spec_id;
	struct tpm_1_vendor vendor;

	struct tpm_1_log_entry entries[]; /* Variable number of entries */
} __packed;

@SergiiDmytruk seems like patches are needed on coreboot master and this behavior affects all measured boot configured platforms. Where can that be tracked?

CC @miczyg1

@tlaurion Not necessarily, we still don't know why this happens. If my guess about something else updating event log is correct, it might be happening due to standard logs, unlike coreboot-specific one, being published in ACPI tables where they can be found by e.g. Linux. If patching src/acpi/acpi.c to do nothing in acpi_create_tcpa() and acpi_create_tpm2() changes anything, then it could be it. However, not publishing the logs seems like a workaround rather than a fix if coreboot doesn't do anything wrong.

@SergiiDmytruk @miczyg1 @krystian-hebel @JonathonHall-Purism I started a discussion at https://matrix.to/#/!pAlHOfxQNPXOgFGTmo:matrix.org/$CQDLxAckANKmXAu8ZDtcDIHUn1dgvgfzyifZidHr7SA?via=matrix.org&via=nitro.chat&via=fairydust.space to ease the process of understanding the current issue at stake tagging peple that should be involved.

EDIT: further documented at #1608.

Issue opened with logs under #1608. This PR doesn't fix anything at all as documented at #1608 (comment)